Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

homebrew_formula_pullrequest.bash: sanitize ~ #1007

Merged
merged 1 commit into from
Sep 18, 2023
Merged

homebrew_formula_pullrequest.bash: sanitize ~ #1007

merged 1 commit into from
Sep 18, 2023

Conversation

scpeters
Copy link
Contributor

@scpeters scpeters commented Sep 7, 2023

Sanitize ~ characters from the brew version string.

Follow up to osrf/homebrew-simulation#2390.

@scpeters
homebrew_formula_pullrequest.bash: sanitize ~
e64920f 
Sanitize ~ characters from the brew version string.

Signed-off-by: Steve Peters <[email protected]>
@scpeters scpeters requested a review from j-rivero as a code owner September 7, 2023 18:56
@scpeters
Copy link
Contributor Author

scpeters commented Sep 7, 2023 •
edited
Loading

testing with Build Status https://build.osrfoundation.org/job/generic-release-homebrew_pull_request_updater/1400/

which is a manual re-run of https://build.osrfoundation.org/job/generic-release-homebrew_pull_request_updater/1391 with the VERSION field changed from 1.0.0~pre2 to 1.0.0~pre3

@scpeters
Copy link
Contributor Author

scpeters commented Sep 7, 2023

testing with Build Status https://build.osrfoundation.org/job/generic-release-homebrew_pull_request_updater/1400/

which is a manual re-run of https://build.osrfoundation.org/job/generic-release-homebrew_pull_request_updater/1391 with the VERSION field changed from 1.0.0~pre2 to 1.0.0~pre3

the resulting pull request ( osrf/homebrew-simulation#2393 ) uses a version string with - instead of ~ as intended, and brew CI appears happy, so I consider this a successful test

@scpeters scpeters mentioned this pull request Sep 7, 2023
Closed
@j-rivero
Copy link
Contributor

j-rivero commented Sep 8, 2023

The point for using ~ in the Debian versions is to get x.y.z~ resolved as lower than x.y.z so we can use x.y.z~pre1 before x.y.z. I can not find any reference about what system is brew using for comparing versions but we would need that behavior to be preserved when changing the ~

@scpeters
Copy link
Contributor Author

scpeters commented Sep 8, 2023

The point for using ~ in the Debian versions is to get x.y.z~ resolved as lower than x.y.z so we can use x.y.z~pre1 before x.y.z. I can not find any reference about what system is brew using for comparing versions but we would need that behavior to be preserved when changing the ~

brew uses custom logic in its version.rb file. I've tested it a bit interactively using brew ruby -e, and it looks like it works. I'll post some output from these commands in a separate comment

@scpeters
Copy link
Contributor Author

scpeters commented Sep 8, 2023 

$ brew ruby -e 'puts Version.new("1.0") == Version.new("1.0")'
true
$ brew ruby -e 'puts Version.new("1.0") == Version.new("1.0.0")'
true
$ brew ruby -e 'puts Version.new("1.0") == Version.new("1.0.0.1")'
false
$ brew ruby -e 'puts Version.new("1.0") == Version.new("1.0-pre1")'
false
$ brew ruby -e 'puts Version.new("1.0") > Version.new("1.0-pre1")'
true

@scpeters
Copy link
Contributor Author

The point for using ~ in the Debian versions is to get x.y.z~ resolved as lower than x.y.z so we can use x.y.z~pre1 before x.y.z. I can not find any reference about what system is brew using for comparing versions but we would need that behavior to be preserved when changing the ~

@j-rivero I believe I've responded to this comment

@j-rivero
Copy link
Contributor

The point for using ~ in the Debian versions is to get x.y.z~ resolved as lower than x.y.z so we can use x.y.z~pre1 before x.y.z. I can not find any reference about what system is brew using for comparing versions but we would need that behavior to be preserved when changing the ~

@j-rivero I believe I've responded to this comment

Thanks for checking, seems fine to me.

@j-rivero j-rivero merged commit 04ad161 into master Sep 18, 2023
1 check passed
@j-rivero j-rivero deleted the scpeters/brew_version_tilda branch September 18, 2023 10:17
@j-rivero j-rivero mentioned this pull request Sep 27, 2023
Merged
@scpeters scpeters mentioned this pull request Oct 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@j-rivero j-rivero Awaiting requested review from j-rivero j-rivero is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@scpeters @j-rivero